Hi @typeid. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

I see the job doesn't run in the PR CI yet, is there a way to test it? I have only tried it locally by using act.

Thanks for the quick review @killianmuldoon.

I think that does make sense, I did not see we already had all kind of verify jobs running in prow prechecks, I was too distracted by the github workflows.

At quick glance, it doesn't seem like the image we're using for the verify job contains plantuml. This would mean we either need it installed in that image, or we run generate-diagrams directly. I'm not sure the latter works (docker in docker?), but I will try to find out.

I'm not sure the latter works (docker in docker?), but I will try to find out.

Ideally we'd simply be able to run verify-diagrams as part of the verify job. That should ensure we have plantuml installed and then run the build. You're right about it probably not working with the docker image though - we'll need to find some solution for that. Do we have Java available in the image we use for the verify jobs?

/ok-to-test

I wasn't able to find java installed in the gcr.io/k8s-staging-test-infra/kubekins-e2e:v20230727-ea685f8747-1.28 image either.

Installing plantuml within the verification image via dnf results in a very old version (PlantUML version 1.2020.02). This old version also produces different images than the version we use (1.2023).

I see a few options here if we want to use the prow job:

• if we can update the k8s-staging-test-infra image to contain plantuml/java, we'd have to always keep the plantuml versions in sync with that image's version.
• if we don't want to update the image, we could still install java and then download the plantuml jar file on every CI run. This would offer more flexibility but bloats the job a lot, and definitely isn't best practice.

For either of these options, we also wouldn't be able to re-use the job calling docker run.

Thanks for adding the ok-to-test, it seems like the git diff failed because it was called in a wrong repository. I'll see if I can fix that in the meantime. :)

/retest

Can we simply run the generate-diagrams target which runs plantuml via Docker? (the verify job already has docker-in-docker enabled)

(+ the usual verify around it)

@sbueringer Thanks for the hint, I did not expect that to work. The latest push should do the trick, sorry for the experimenting!

I also noticed that quite a few verify-* jobs aren't run in the verify make target, notably:

• verify-security
• verify-govulncheck
• verify-container-images

See:

ALL_VERIFY_CHECKS = licenses boilerplate shellcheck tiltfile modules gen conversions doctoc capi-book-summary diagrams

.PHONY: verify
verify: $(addprefix verify-,$(ALL_VERIFY_CHECKS)) lint-dockerfiles ## Run all verify-* targets

Is this intended? I believe they're not run in CI at the moment.

I also noticed that quite a few verify-* jobs aren't run in the verify make target

This is intended - these scans are currently not run on PRs and are instead run as part of the weekly security scan: https://github.com/kubernetes-sigs/cluster-api/blob/main/.github/workflows/weekly-security-scan.yaml

Error looks good:

make generate-diagrams-proposals
make[1]: Entering directory '/home/prow/go/src/sigs.k8s.io/cluster-api'
docker run -v /home/prow/go/src/sigs.k8s.io/cluster-api/docs:/docs  plantuml/plantuml:1.2023 /docs/proposals/**/*.plantuml
make[1]: Leaving directory '/home/prow/go/src/sigs.k8s.io/cluster-api'
diff --git a/docs/proposals/images/machine-health-check/mhc.png b/docs/proposals/images/machine-health-check/mhc.png
index 395a7bba2..a50e002f6 100644
Binary files a/docs/proposals/images/machine-health-check/mhc.png and b/docs/proposals/images/machine-health-check/mhc.png differ
generated diagrams are out of date, run make generate-diagrams
make: *** [Makefile:679: verify-diagrams] Error 1

If you revert that commit I think this one is good to go!

LGTM label has been added.

/retitle 🌱 Verify plantuml image generation in CI

Thank you very much!!

Pretty happy that we don't have to deal with Java 😂

/approve

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here